Adding initial draft of the Containerz yang model#1389
Adding initial draft of the Containerz yang model#1389alshabib wants to merge 1 commit intoopenconfig:masterfrom
Conversation
3ea4065 to
3f56831
Compare
|
|
||
| augment "/oc-sys:system/oc-sys-grpc:grpc-servers/oc-sys-grpc:grpc-server/" + | ||
| "oc-sys-grpc:state" { | ||
| when "../config[contains(services, 'oc-containerz:CONTAINERZ')]/enable = 'true'"; |
There was a problem hiding this comment.
This identity does not exist. For new gRPC services, there is current way and potential future for expansion but you'll want to at a min follow what you should see for P4RT and GRIBI
| "A collection of counters that were collected while evaluating | ||
| access to the gRPC server."; | ||
|
|
||
| container counters { |
There was a problem hiding this comment.
This appears to try to add an overlapping container under /system/grpc-servers/grpc-server/state. Suggest consideration of bucketizing "containerz" specific state/counters under it's own hierarchy specific to this set of services
There was a problem hiding this comment.
| "A collection of counters that were collected by the gRPC during | ||
| the authentication process."; | ||
|
|
||
| leaf connection-rejects { |
There was a problem hiding this comment.
What are these connection-* related leafs meant to convey or were they copied from other model?
There was a problem hiding this comment.
They seem to be a duplicate of the counters in /system/grpc-servers/grpc-server/connections/connection/state/counters. I think they are not needed because they are already present.
/system/grpc-servers/grpc-server/containerz/state/counters could be created if there are containerz specific counters. But all generic "grpc-servers" counters regarding client connections should appear in:
/system/grpc-servers/grpc-server/connections/connection/state/counters
| description | ||
| "gRPC server containerz freshness-related data."; | ||
|
|
||
| container containers { |
There was a problem hiding this comment.
Similar suggestion as above - consider grouping under containerz hierarchy first. If this is meant to be similar to docker ps outputs then can align as such. Any counters should go under child counters container at relevant hierarchy.
|
|
||
| augment "/oc-sys:system/oc-sys-grpc:grpc-servers/oc-sys-grpc:grpc-server/" + | ||
| "oc-sys-grpc:state" { | ||
| when "../config[contains(services, 'oc-containerz:CONTAINERZ')]/enable = 'true'"; |
There was a problem hiding this comment.
We don't allow for this kind of when statement using contains AFAIK.
There was a problem hiding this comment.
This concept was introduced in the gNSI related models
https://github.com/openconfig/public/blob/master/release/models/gnsi/openconfig-gnsi-authz.yang#L205
But this identity does not exist and each gRPC service domain should really bring in a distinct related container and group everything related underneath (commented above)
| "gRPC server containerz freshness-related data."; | ||
|
|
||
| container containers { | ||
| list container { |
There was a problem hiding this comment.
I see this as duplicative to the ListContainer RPC, which is fine in principle, but how do we expect the two to be used? I presume the thinking here is that some clients will just subscribe through gNMI to understand this state?
| "A timestamp of the last time a gRPC client succeeded | ||
| in establishing a connection to the server."; | ||
| } | ||
| leaf running-containers { |
There was a problem hiding this comment.
Is this a useful metric if we also have the list of containers?
|
/gcbrun |
|
No major YANG version changes in commit 6d0ec50 |
| "A collection of counters that were collected by the gRPC during | ||
| the authentication process."; | ||
|
|
||
| leaf connection-rejects { |
There was a problem hiding this comment.
They seem to be a duplicate of the counters in /system/grpc-servers/grpc-server/connections/connection/state/counters. I think they are not needed because they are already present.
/system/grpc-servers/grpc-server/containerz/state/counters could be created if there are containerz specific counters. But all generic "grpc-servers" counters regarding client connections should appear in:
/system/grpc-servers/grpc-server/connections/connection/state/counters
| "A collection of counters that were collected while evaluating | ||
| access to the gRPC server."; | ||
|
|
||
| container counters { |
There was a problem hiding this comment.
| list container { | ||
| key "name"; | ||
|
|
||
| leaf name { |
There was a problem hiding this comment.
For openconfig style compliance, this needs to be a leafref to a leaf of the same name in a "state container"
for example:
public/release/models/system/openconfig-system-grpc.yang
Lines 75 to 129 in 1463f23
| description | ||
| "gRPC server containerz freshness-related data."; | ||
|
|
||
| container containers { |
There was a problem hiding this comment.
Consider the same fields from a docker ps (or docker ls if the intent is only to show running containers)?
|
|
||
| // Augments section. | ||
|
|
||
| augment "/oc-sys:system/oc-sys-grpc:grpc-servers/oc-sys-grpc:grpc-server/" + |
There was a problem hiding this comment.
I'm not sure if we really should be augmenting the grpc-servers list with information about something which is system-level (i.e. containers are not tied to a particular gRPC server in any way).
maybe something like /system/containers would be more appropriate?
There was a problem hiding this comment.
(likewise for the augment below)
|
FYI: set project review status to "waiting for author". |
Change Scope
This is a yang model for the gNOI.Containerz service.
This change is backwards compatible.